Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tenants UI #8015

Merged
merged 22 commits into from
Dec 17, 2020
Merged

Tenants UI #8015

merged 22 commits into from
Dec 17, 2020

Conversation

agriffard
Copy link
Member

@agriffard agriffard commented Dec 15, 2020

  • Badge state: icon + text
  • Buttons icon with tooltips
  • Open external link
  • Filter on state click
  • Display url host (can have multiple domains)
  • Database and Recipe badges
  • Vertically align description with title

image

@hishamco
Copy link
Member

Bigger state icon

It was bigger at the begging ;)

@agriffard agriffard requested a review from hishamco December 16, 2020 00:14
@Skrypt
Copy link
Contributor

Skrypt commented Dec 16, 2020

Try the state filters you will see that there is something to fix when no result is found.

@deanmarcussen
Copy link
Member

I don't think we should be using dots w/colors to indicate things.

This applies more to the admin listing, but the patterns should be similar everywhere.

I think we should be using coloured icons to represent states.

It's gone a little too minimilist to get good quick readability.

So suggestion is same colors, apply an icon to the color scheme.

Example from devops. Ticks and Crosses

Screenshot 2020-12-16 at 09 30 17

Second suggestion. The buttons are quite small. A little bigger maybe?

@deanmarcussen
Copy link
Member

Prefer the tags w/badges 👍

(it helps keep the ui extensible easier as well)

Think the Open action was better on the right

Maybe not bold text is better for the actions Running, not sure.

Whats the lowercase a mean?

@Skrypt
Copy link
Contributor

Skrypt commented Dec 16, 2020

lowercase a is the tenant description written quickly like asdf

@agriffard
Copy link
Member Author

4px margin added.

@Skrypt
Copy link
Contributor

Skrypt commented Dec 16, 2020

Sans titre

@agriffard
Copy link
Member Author

Ok for the margin.
Why don't you have a disable icon on the running tenant?

@Skrypt
Copy link
Contributor

Skrypt commented Dec 16, 2020

I think it's by design no?

image

Though, I don't get the recipe name from the default tenant.

@Skrypt
Copy link
Contributor

Skrypt commented Dec 16, 2020

Ok I like it

@Skrypt
Copy link
Contributor

Skrypt commented Dec 16, 2020

Maybe some work on the badges colors. I'm trying badge-secondary and font color white. It works on both darkmode and lightmode. But what you did works too. It's just really light.

@Skrypt
Copy link
Contributor

Skrypt commented Dec 16, 2020

The disable tenant button requires a confirm modal.

@scleaver
Copy link
Contributor

scleaver commented Dec 16, 2020

All list UIs aren't moving this "icon" based direction are they? Personally I am not a fan of too many icons for actions as they require interpretation by the user.

If so can you apply the icons via a class applied to the button rather than inserting the icon markup into the button itself... this way those that want to can remove the icons via CSS rather than having to use a view in our theme to override

Also for a saas installation are you really going to have different tenants on different database types - seems unnecessary?

@Skrypt
Copy link
Contributor

Skrypt commented Dec 16, 2020

I kind of agree that icons only are quite small compared with buttons.
Maybe buttons with icons would be better.
Easier for fat fingers to click them. 🙈
Or bigger icons?

If I would use SaSS I'd probably still want to use SQLite and a second one most of the time.

image

There is a toolltip now.

@Skrypt
Copy link
Contributor

Skrypt commented Dec 16, 2020

Here I exagerated a little :

image

@Skrypt
Copy link
Contributor

Skrypt commented Dec 16, 2020

And here I think it needs some adjustment for mobile :

image

@Skrypt
Copy link
Contributor

Skrypt commented Dec 16, 2020

Sans titre

This is why a margin-left is not enough. It needs flexboxes.

@agriffard
Copy link
Member Author

@scleaver Icons have a Bootstrap tooltip with the same text they used to have.
I will only change the content items and the tenants for the moment.

@agriffard
Copy link
Member Author

2 columns as the content items:

image

@Skrypt
Copy link
Contributor

Skrypt commented Dec 17, 2020

Fixed margin with Flexbox.
Adding modal confirm when disabling a tenant.
Added text breaking if there is anyone using long tenant names.
Merged latest dev and fixed merge issue.

image

@agriffard agriffard merged commit 442af5a into dev Dec 17, 2020
@delete-merged-branch delete-merged-branch bot deleted the ag/tenantsTooltips branch December 17, 2020 21:43
@Skrypt Skrypt added the breaking change 💥 Issues or pull requests that introduces breaking change(s) label Dec 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change 💥 Issues or pull requests that introduces breaking change(s)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants